@K20shores @baburton do you think this change is a good & proper fix long term?

What's your opinion on the stale #4072?

Basically what I'm asking:

• What's the best solution long-term in your opinions?
• What's actually practical, and what are the trade-offs, short and long term?

I really have no idea at this minute.

How much trouble is it to add one Windows ARM job to the CI here?

@K20shores @baburton do you think this change is a good & proper fix long term?

In my patch I add __attribute__((visibility("default"))) alongside the existing __declspec(dllexport) exactly when building under windows/arm64. This was an attempt to be as targeted as possible without affecting other platforms. However, on further thought I suspect what you actually want is to do this exactly when building under windows and using gcc-like compilers.

The reasons:

• I have zero experience with MSVC++, but it's not clear to me that the Microsoft compiler supports __attribute__ at all (and I do see that other uses of __attribute__ in pybind11 are targeted to gcc/clang). In particular, pybind11 only applies hidden visibility by default when __GNUG__ is defined (at the bottom of pybind11_namespace_macros.h), so presumably this problem never appears under the Microsoft compiler anyway.
• I don't think my patch is specific to arm64 (originally I was trying to be as precisely targeted as possible, to unbreak pybind11 on windows/arm64/clang without affecting other working platforms). I am no expert on how visibility flags interact with the DLL export manifest; however, it would seem to me that if an entity is in the DLL export manifest (i.e., part of the public DLL interface) then surely it should be visible. So I cannot see the harm in allowing this patch to apply to all windows gcc-like builds.

So: I expect that where my patch used #if defined(__aarch64__), what you really want is #if defined(__GNUG__).

This would expand the patch to cover windows/x64 builds that use msys2 with gcc or clang. Unfortunately I cannot test this myself, since msys2 does not install on my windows x64 VMs. From my past experience, windows/x64/gcc was already working, and I had never tried windows/x64/clang. (The build failure being discussed here was on windows/arm64/clang, and msys2 does not provide a windows/arm64/gcc option at all).

What's your opinion on the stale #4072?

Issue #4072 appears to be about widespread changes to the visibility attributes used in pybind11 - for this I have no informed opinion I'm afraid (I don't know enough about the pros, cons and behaviour of different visibility flags under different linkers on different platforms).

Oh, wow ... there seems to be a lot more uncertainty than I assumed originally.

We need a least one firm use case to make a change here. A minimum would be logs to show what the original problem was on a well-defined platform. Much better would be to

• add a job in .github/workflows/ci.yml,
• run the CI so that we see the failure reproduced,
• add a fix to resolve the failure.

@rwgk To be clear: pybind11 is indeed currently broken on windows/arm64/msys2+clang (you can see the build error in #5883, and both @K20shores and myself have experienced this exact error), and adding __attribute__((visibility("default"))) to PYBIND11_EXPORT does indeed fix it. This has been explicitly tested and confirmed.

The uncertainty you mention was my attempt to predict how this patch will affect other platforms, most of which I am not able to test myself (like you, I port software to windows; I am not a native windows developer).

I am not familiar with the GitHub CI system, or how pybind11 uses it, so this is something I will not be able to help with. However, if your CI system includes a windows/arm64/msys+clang platform then you should be able to see both the failure before the patch and the fix after the patch. If your CI system includes other platforms then you should be able to see what I am just predicting about the suggested patch (i.e., that it does not break other platforms).

To be clear, the patch I'm suggesting is the one attached below, which hides the (very simple) change behind compile-time tests for windows and a GNU-like compiler (so gcc or clang). This should clearly not affect other platforms (e.g., linux or macOS) or other compilers (e.g., MSVC++), and so if your CI system includes other windows/msys setups (e.g., windows/x86/msys+mingw64 and windows/x86/msys+clang) then this should give you strong confidence in the patch.

I've attached the patch again here, including my edits above (so replacing __aarch64__ with __GNUG__).

pybind11.diff.txt

@rwgk @baburton I'm working on adding a CI step for windows-11-arm, both with and without mingw. I'll submit that as a separate PR once I get something running and then we can rebase this branch onto that, with @baburton's latest (likely more appropriate) patch.

@rwgk @baburton I'm working on adding a CI step for windows-11-arm, both with and without mingw. I'll submit that as a separate PR once I get something running and then we can rebase this branch onto that, with @baburton's latest (likely more appropriate) patch.

Sounds awesome, that’s exactly what we need to keep things clean and well-tested here.